Skip to content

Close out the remaining small note-tier CodeQL categories - #142

Open
vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:note-small-categories
Open

vharseko wants to merge 2 commits into
OpenIdentityPlatform:masterfrom
vharseko:note-small-categories

Conversation

@vharseko

@vharseko vharseko commented Sep 19, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to the unused-parameter cleanup (#141): investigated all the newly-surfaced small note-severity CodeQL categories.

  • GuardedString now overrides toString() (returns "GuardedString(...)"), so it no longer inherits Object's default. Fixes call-to-object-tostring at its root instead of patching the two current call sites (SharedSecretPrincipal, ScriptOnResourceApiOpTests) individually — any future logging of a GuardedString is safe by construction.
  • Removes the dead ContractTestFactory inner class from ContractITCase (unused-reference-type) and its now-unused imports.
  • Fixes a shadowed local in LdapInternalSearch.execute() (local-shadows-field).

Also dismissed on GitHub: 3 ignored-error-status-of-call (the ignored return values are already covered by a subsequent check or exception path), 8 jdk-internal-api-access (AD DirSync's com.sun.jndi.ldap.Ber*, no public JDK alternative exists), 1 confusing-method-signature (Log.log(..) is long-standing, heavily-used public API — renaming to remove the overload is too invasive for the benefit).

Update 2026-09-21: dropped AttributeTypeUtil.java, MultiOpTests.java, PrettyStringBuilder.java, StringUtil.java, ActiveDirectoryChangeLogSyncStrategy.java, XSDAnnotationParser.java and the AttributeTypeUtilTests.java test from this PR — #134 (opened independently, earlier) already fixes those exact same lines byte-for-byte. uncaught-number-format-exception, inefficient-boxed-constructor, inefficient-empty-string-test, inefficient-key-set-iterator, unknown-javadoc-parameter and missing-space-in-concatenation are covered there instead; this PR now only carries the 3 fixes unique to it.

Test plan

  • New test: GuardedStringTests.testToStringNeverExposesTheClearText.
  • mvn install on connector-framework, connector-framework-contract, OpenICF-ldap-connector (incl. existing test suites, LDAP/OpenDJ integration tests included) — all green, 946 tests, 0 failures.

- GuardedString now overrides toString() so it never inherits Object's
  default (fixes call-to-object-tostring at its two call sites in one
  place instead of patching each site).
- AttributeTypeUtil.createInstantiatedObject wraps its numeric parsing
  in the same try/catch -> ConnectorException pattern used elsewhere
  (uncaught-number-format-exception), and switches from deprecated
  boxed constructors to the static parse methods.
- Remove the dead ContractTestFactory inner class and its now-unused
  imports.
- Mechanical fixes: StringUtil/XSDAnnotationParser empty-string checks,
  PrettyStringBuilder's Map iteration via entrySet(), a shadowed local
  in LdapInternalSearch, a javadoc @PARAM typo/gap in MultiOpTests, and
  a missing space in a log message in ActiveDirectoryChangeLogSyncStrategy.
@vharseko vharseko added java Pull requests that update java code framework OpenICF-java-framework connector:ldap LDAP connector connector:xml XML connector tests Test additions or fixes bug Something isn't working labels Sep 19, 2026
AttributeTypeUtil.java, MultiOpTests.java, PrettyStringBuilder.java,
StringUtil.java, ActiveDirectoryChangeLogSyncStrategy.java and
XSDAnnotationParser.java were independently fixed here and in OpenIdentityPlatform#134
with byte-identical diffs; reverting them here to avoid merging the
same change twice and to let OpenIdentityPlatform#134 own them. Drops
AttributeTypeUtilTests.java too since it exercises the
NumberFormatException-wrapping behavior that lived in the now-reverted
AttributeTypeUtil.java (still present in OpenIdentityPlatform#134, just not in this PR
anymore).
@vharseko vharseko removed the connector:xml XML connector label Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working connector:ldap LDAP connector framework OpenICF-java-framework java Pull requests that update java code tests Test additions or fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant